feat(web): Organization page with "standalone" theme - Level 2 sitemap #17085
Conversation
WalkthroughThe pull request introduces a new sitemap type, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17085 +/- ##
==========================================
- Coverage 35.68% 35.66% -0.02%
==========================================
Files 6946 6947 +1
Lines 147401 147512 +111
Branches 41904 41938 +34
==========================================
+ Hits 52600 52615 +15
- Misses 94801 94897 +96
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 11 Total Test Services: 0 Failed, 10 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (5) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
apps/web/screens/Organization/Standalone/Level2Sitemap.tsx (1)
109-176: Consider using Next.js data fetching methods for better alignment with best practices.The
getPropsmethod attached to theStandaloneLevel2Sitemapcomponent is unconventional in Next.js. Typically, data fetching is performed usinggetStaticPropsorgetServerSidePropsat the page level. Refactoring to usegetStaticPropswould adhere to Next.js best practices for static generation and improve clarity.apps/web/pages/s/[...slugs]/index.tsx (1)
336-347: Consider enhancing error handling with logging.While the error handling correctly differentiates between CustomNextError and other exceptions, consider adding error logging for better observability.
try { return { page: { type: PageType.STANDALONE_LEVEL2_SITEMAP, props: await StandaloneLevel2Sitemap.getProps(modifiedContext), }, } } catch (error) { + console.error('Failed to get Level 2 sitemap props:', error) if (!(error instanceof CustomNextError)) { throw error } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
libs/cms/src/lib/generated/contentfulTypes.d.tsis excluded by!**/generated/**
📒 Files selected for processing (8)
apps/web/pages/s/[...slugs]/index.tsx(5 hunks)apps/web/screens/Organization/Standalone/Level2Sitemap.tsx(1 hunks)apps/web/screens/queries/Organization.tsx(1 hunks)libs/cms/src/lib/cms.contentful.service.ts(3 hunks)libs/cms/src/lib/cms.resolver.ts(2 hunks)libs/cms/src/lib/dto/getOrganizationPageStandaloneSitemap.input.ts(1 hunks)libs/cms/src/lib/models/organizationPageStandaloneSitemap.model.ts(1 hunks)libs/cms/src/lib/models/organizationParentSubpage.model.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
libs/cms/src/lib/dto/getOrganizationPageStandaloneSitemap.input.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
apps/web/screens/Organization/Standalone/Level2Sitemap.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
libs/cms/src/lib/models/organizationParentSubpage.model.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
apps/web/screens/queries/Organization.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/pages/s/[...slugs]/index.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
libs/cms/src/lib/models/organizationPageStandaloneSitemap.model.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/cms/src/lib/cms.resolver.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/cms/src/lib/cms.contentful.service.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (6)
libs/cms/src/lib/dto/getOrganizationPageStandaloneSitemap.input.ts (1)
21-25: Input type extension is well-defined and appropriate.
The GetOrganizationPageStandaloneSitemapLevel2Input class correctly extends the GetOrganizationPageStandaloneSitemapLevel1Input and adds the required subcategorySlug field, enhancing the input schema for level 2 sitemaps.
libs/cms/src/lib/models/organizationPageStandaloneSitemap.model.ts (1)
22-50: New GraphQL object types are well-defined and enhance the sitemap model.
The addition of OrganizationPageStandaloneSitemapLevel2Link, OrganizationPageStandaloneSitemapLevel2Category, and OrganizationPageStandaloneSitemapLevel2 effectively extends the sitemap model to support level 2 sitemaps, providing a more detailed hierarchical structure.
libs/cms/src/lib/models/organizationParentSubpage.model.ts (1)
38-39: Filter enhancement ensures correct association of pages.
The added condition in the filter method verifies that each page's organizationPage ID matches the parent organizationPage ID. This ensures that only pages belonging to the correct organization are included, improving data accuracy.
apps/web/screens/queries/Organization.tsx (1)
449-465: Well-structured GraphQL query for hierarchical sitemap data!
The query is well-designed with a clear hierarchical structure that properly represents the sitemap levels through nested fields (label, childCategories, childLinks).
apps/web/pages/s/[...slugs]/index.tsx (1)
330-349: Well-implemented routing logic for Level 2 sitemap!
The implementation:
- Correctly validates the standalone theme
- Properly checks top-level navigation links
- Has appropriate error handling
- Falls back to parent subpage when needed
libs/cms/src/lib/cms.resolver.ts (1)
739-747: Clean and consistent resolver implementation!
The implementation:
- Uses appropriate cache control
- Has proper typing for input and return values
- Follows the established patterns in the codebase
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
libs/cms/src/lib/cms.contentful.service.ts (1)
1288-1435: Consider refactoring repeated patterns in sitemap traversal.The implementation is thorough but contains repeated patterns for handling different node types. Consider extracting common logic into helper functions:
- Node type checking and transformation
- Entry node handling
- URL construction
Example refactor for node transformation:
+private transformSitemapNode( + node: SitemapTreeNode, + entryNodes: Map<string, any[]>, + input: GetOrganizationPageStandaloneSitemapLevel2Input +) { + if (node.type === SitemapTreeNodeType.CATEGORY) { + return { + label: node.label, + childLinks: node.childNodes.map(childNode => + this.transformSitemapNode(childNode, entryNodes, input) + ), + } + } + if (node.type === SitemapTreeNodeType.URL) { + return { + label: node.label, + href: node.url, + childLinks: [], + } + } + return this.createEntryNode(node.entryId, entryNodes) +}This would make the code more maintainable and reduce duplication.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
libs/cms/src/lib/cms.contentful.service.ts(3 hunks)libs/cms/src/lib/models/organizationParentSubpage.model.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/cms/src/lib/models/organizationParentSubpage.model.ts
🧰 Additional context used
📓 Path-based instructions (1)
libs/cms/src/lib/cms.contentful.service.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (5)
libs/cms/src/lib/cms.contentful.service.ts (5)
91-98: LGTM: Import statements are well-organized.
The new imports for sitemap types and models are properly structured.
1228-1228: LGTM: URL construction and data validation improvements.
The changes correctly:
- Enhance URL construction by incorporating categorySlug
- Add proper data validation by pruning empty values
Also applies to: 1280-1282
1300-1300: Fix the error handler name.
The error handler name should be 'getOrganizationPageStandaloneSitemapLevel2' instead of 'getOrganizationPageStandaloneSitemapLevel1'.
1304-1326: LGTM: Robust null checking and type validation.
The implementation properly validates:
- Tree existence
- Category existence
- Subcategory existence and type
- Edge cases with proper null returns
This ensures type safety and prevents runtime errors.
1424-1433: LGTM: Comprehensive data cleanup.
The pruning logic effectively removes invalid entries by:
- Filtering out links without required fields
- Ensuring categories have valid labels and non-empty child links
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
libs/cms/src/lib/cms.contentful.service.ts (1)
Line range hint
1228-1286: Consider using URL builder utility for consistent path construction.The URL construction is spread across multiple string interpolations. Consider extracting this into a utility function to ensure consistent URL construction and make it easier to update path patterns.
Example implementation:
+ const buildOrganizationUrl = (lang: string, organizationSlug: string, ...segments: string[]) => + `/${getOrganizationPageUrlPrefix(lang)}/${organizationSlug}/${segments.join('/')}`; - href: `/${getOrganizationPageUrlPrefix(input.lang)}/${input.organizationPageSlug}/${input.categorySlug}/${node.slug}`, + href: buildOrganizationUrl(input.lang, input.organizationPageSlug, input.categorySlug, node.slug),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
apps/web/pages/s/[...slugs]/index.tsx(5 hunks)libs/cms/src/lib/cms.contentful.service.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/pages/s/[...slugs]/index.tsx
🧰 Additional context used
📓 Path-based instructions (1)
libs/cms/src/lib/cms.contentful.service.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (2)
libs/cms/src/lib/cms.contentful.service.ts (2)
91-98: LGTM: Import statements are well-organized.
The new imports for sitemap-related types and models are properly structured.
1300-1300: Correct the error handler name in the catch block.
The error handler name should match the method name for accurate error logging.
kodiakhq
bot
deleted the
feature/web-organization-page-standalone-sitemap-level2
branch
Organization page with "standalone" theme - Level 2 sitemap
What
Level 2 sitemap added (/s/"organization"/"category"/"subcategory")
Screenshots / Gifs
Screen.Recording.2024-12-02.at.14.41.49.mov
Checklist:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
STANDALONE_LEVEL2_SITEMAP, enhancing navigation for organization pages.StandaloneLevel2Sitemapcomponent for improved sitemap rendering and user interface.Bug Fixes
Documentation